Skip to content

Feat implement get 3d meshes #504

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 6 commits into from
Nov 19, 2025
Merged

Feat implement get 3d meshes #504

merged 6 commits into from
Nov 19, 2025

Conversation

@mattmacf98
Copy link
Member

@mattmacf98 mattmacf98 commented Nov 18, 2025

Description

https://viam.atlassian.net/browse/APP-14391
This PR implements a new Get3DModels api (approved scope doc: https://docs.google.com/document/d/1jOQYh6OAsK09GhXWCqigGT739apX96RFcmTtFvyVMnE/edit?ouid=111066372492921892069&usp=docs_home&ths=true) in the cpp sdk

Testing

  • added new functionality to the mock arm and wrote a unit test to confirm it returned the 3d model map as expected

@mattmacf98 mattmacf98 requested a review from a team as a code owner November 18, 2025 15:07
@mattmacf98 mattmacf98 requested review from lia-viam and njooma and removed request for a team November 18, 2025 15:07
@lia-viam
Copy link
Collaborator

lia-viam commented Nov 18, 2025

@mattmacf98 thanks for the PR!

I've written you a struct mesh in viam/sdk/common/mesh.xpp. Can you replace all instances of common::v1::Mesh with this struct, and remove the API include from arm.hpp? Note that you can call to_proto(viam::sdk::mesh) and from_proto(viam::common::v1::Mesh) to do conversions in the arm_client.cpp and arm_server.cpp

In the C++ SDK we do not allow the use of any protobuf generated API types (found in the viam/api directory) in public headers. This lets us keep grpc/protobuf/abseil as a private dependency of the SDK and also saves us from working with the proto gencode which is a nightmare to use. All of this should definitely be in a subheading in the README somewhere.

lia-viam
lia-viam reviewed Nov 18, 2025
View reviewed changes
src/viam/sdk/components/arm.hpp Outdated
/// @brief Defines an `Arm` component
#pragma once

#include <common/v1/common.pb.h>
Copy link
Collaborator

@lia-viam lia-viam Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the include you'll want to delete, and add mesh.hpp to the include block below

lia-viam
lia-viam requested changes Nov 18, 2025
View reviewed changes
Copy link
Collaborator

@lia-viam lia-viam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see other comment about using struct mesh

@mattmacf98
Copy link
Member Author

mattmacf98 commented Nov 18, 2025

Thank you for adding that Mesh struct @lia-viam!! I updated the files and used the conversion logic for the client (from_proto for outputting data) and server (to_proto for sending data back to client)

@mattmacf98 mattmacf98 requested a review from lia-viam November 18, 2025 22:34
@mattmacf98 mattmacf98 merged commit 9427ed1 into main Nov 19, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lia-viam lia-viam lia-viam approved these changes

@njooma njooma Awaiting requested review from njooma njooma is a code owner automatically assigned from viamrobotics/sdk-netcode

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@mattmacf98 @lia-viam